-
Notifications
You must be signed in to change notification settings - Fork 181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf: impl put_many and reuse column families #412
Conversation
6f18ec3
to
a47d4ba
Compare
03a8fe1
to
330e7b6
Compare
|
||
#[tracing::instrument(skip(self))] | ||
pub async fn get(&self, cid: &Cid) -> Result<Option<DBPinnableSlice<'_>>> { | ||
self.get0(cid, &self.cfs()?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about using `spawn_blocking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I noticed this as well now that I removed all the async. It was not using spawn_blocking before, but it probably should.
But I got a branch where I really clean things up, so I would prefer to introduce spawn_blocking there. https://github.com/n0-computer/iroh/tree/rklaehn/local-gb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
This requires getting rid of a lot of async in the store. Async only in the public interface
90bba13
to
a193ab4
Compare
This branch:
Main:
|
@rklaehn did you switch up things? or does this make things slower? |
If Im reading the above correct, it's 1 sec faster but uses more CPU cores (hence user time higher) |
This requires getting rid of a lot of async in the store. Async only in the public interface.
I want to do some more reorg here, but in the interest of getting something on main soon I will refrain from it...